JSDoc completions #1561

gabritto · 2025-08-11T23:05:08Z

Port of JSDoc completions. I think nothing had to be changed compared to Strada.

This also includes some bug fixes, mainly a change to astnav.VisitEachChildAndJSDoc so that it takes visiting functions and wraps them in a way that skips visiting a JSDoc node's comments when that node has a single comment.
The reason for this is that in Strada, that single JSDoc comment used to be encoded as a comment property of type string, and during forEachChild we would not visit that string (because we can only visit nodes), and that also meant that node.getChildren() would not include that single JSDoc comment.
In Corsa, every comment is a node. There is no comment property of type string, so we can in fact visit that single comment. But code in services relies on that special behavior, so I added it directly to astnav.VisitEachChildAndJSDoc (in the right way this time, instead of the partial fix I had in place before).

internal/ls/completions.go

Copilot

Pull Request Overview

This PR ports JSDoc completions functionality from the TypeScript compiler to provide auto-completion support for JSDoc comments and tags. The main purpose is to enable IntelliSense-like features when writing JSDoc documentation, including completion for tag names, parameter names, and type expressions.

Key changes include:

Implementation of JSDoc completion logic with support for tag names, parameters, and type expressions
Bug fixes to AST navigation for JSDoc single comment nodes to match TypeScript's behavior
Updates to string completion handling to return the original item instead of null for better fallback behavior

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`internal/ls/completions.go`	Core JSDoc completion implementation with new completion data types and handlers
`internal/astnav/tokens.go`	Enhanced `VisitEachChildAndJSDoc` with proper JSDoc single comment node handling
`internal/ast/utilities.go`	New utility functions for JSDoc node identification and comment handling
`internal/ast/ast.go`	Added AST node methods for accessing JSDoc properties and type expressions
`internal/lsutil/children.go`	Updated to use new `VisitEachChildAndJSDoc` signature
`internal/ls/utilities.go`	Updated visitor usage and added reparsed node flag check
`internal/ls/string_completions.go`	Changed to return original item instead of nil for better fallback
`internal/format/util.go`	Exported `GetLineStartPositionForPosition` function
`internal/checker/printer.go`	Added `TypeToTypeNode` method for type-to-AST conversion
Test files	Enabled previously skipped JSDoc completion tests and added new basic test

jakebailey · 2025-08-13T02:34:51Z

But code in services relies on that special behavior, so I added it directly to astnav.VisitEachChildAndJSDoc (in the right way this time, instead of the partial fix I had in place before).

Is there a chance we need the old non-special case way?

jakebailey · 2025-08-13T02:35:43Z

internal/ast/utilities.go

 }

 // True if node is of a JSDoc kind that may contain comment text.
+// NOTE: while this is a faithful port of Strada's implementation,


Should Comments just check this and return nil or something?

That's not the way it used to work in Strada. You can access .comments on the node even if .comments is a string, but you wouldn't visit them in Strada's forEachChild.

Is there a chance we need the old non-special case way?

I think we might need a version that visits all things. e.g. in Corsa,ForEachChild is used in the binder and so probably needs to visit single comments in JSDoc. We may be able to get away with adding the special behavior to Corsa's VisitEachChild, but I'm not sure.

@sandersn might have a better idea.

stupid suggestion because I just started thinking about this, but: JSDocTypeLiteral and JSDocTypeSignature are the bodies of a typedef and a callback respectively. The reason they don't have comments is that their parent has the comment instead. Why do they return true for this function? Is it because they contain children that could themselves contain comments?

This function is only called in two places, neither of which appear to deal with jsdoc comment text, but finding nodes from a position. My best guess is that the function needs to be renamed so that people don't think Comment() is safe to call on nodes it returns true from.

Oh, I got totally confused in my original answer, I thought Jake's comment was about a different function. But I agree we should rename it, I just don't know to what. In Strada this function even has a comment saying "True if node is of a kind that may contain comment text", so maybe this function was wrong in Strada too? I'm not sure. Maybe we should rename to DoNotUseVeryBadIsJSDocCommentContainingNode 😅

I renamed it and also moved it to the astnav package, the only place where it's used. Couldn't think of a good name because I don't really know what the function is supposed to do.

internal/format/util.go

internal/ls/completions.go

internal/checker/printer.go

sandersn

Minor suggestions and questions, but I think this is a faithful port of some slightly odd code.

internal/ast/ast.go

sandersn · 2025-08-13T21:31:03Z

internal/ast/utilities.go

 }

 // True if node is of a JSDoc kind that may contain comment text.
+// NOTE: while this is a faithful port of Strada's implementation,


stupid suggestion because I just started thinking about this, but: JSDocTypeLiteral and JSDocTypeSignature are the bodies of a typedef and a callback respectively. The reason they don't have comments is that their parent has the comment instead. Why do they return true for this function? Is it because they contain children that could themselves contain comments?

This function is only called in two places, neither of which appear to deal with jsdoc comment text, but finding nodes from a position. My best guess is that the function needs to be renamed so that people don't think Comment() is safe to call on nodes it returns true from.

sandersn · 2025-08-13T21:42:11Z

internal/fourslash/tests/gen/jsdocExtendsTagCompletion_test.go

 func TestJsdocExtendsTagCompletion(t *testing.T) {
 	t.Parallel()
-	t.Skip()
+


the original test is weird -- we expect to have A as a completion for A extends /**/ ? I guess it's the only class around, but it still doesn't make sense.

internal/ls/completions.go

sandersn · 2025-08-13T22:35:43Z

internal/ls/completions.go

+		t := "*"
+		if isObject {
+			// !!! Debug.assert(!dotDotDotToken, `Cannot annotate a rest parameter with type 'Object'.`);
+			t = "Object"


wait, what? Object is somehow the worst type for anything in either TS or JS. In Strada it means any if strict is off but in Corsa it always means Object. I think object makes more sense, or * or any if object is nonsensical somehow.

Yeah, I can't really remember why I used Object here originally. Maybe that's what I more commonly saw in JS at the time? Anyways, I'll update to object.

wait wait. I remembered. It's probably because it's preparing to emit nested object literal type notation:

/** * @param {Object} param0 * @param {string} param0.a * @param {number} param0.b */ function f({ a, b }) {}

So it might make sense.
It's also one more reason why Object -> any in Strada, even though I hate that and it's not coming back for Corsa.

As discussed offline, this works with object too:

https://www.typescriptlang.org/play/?ts=6.0.0-dev.20250814&filetype=js#code/PQKhAIChwgBAHAhgJ0QW3AbwPYCMBWApgMYAuAvuEqmgAzRzXpYDOpyAlgHYDmlTdAHSIG4BCmaYuAVzS5CyfhKG4GwSADNpXMh2xdwGgBSZwiADThc4ALzgAjOHIBKLNHAfi+ltgA2hQV9sHiNEZwBudw9kQlJpZANHAGorSPIgA

So keeping object until someone complains.

Co-authored-by: Nathan Shively-Sanders <[email protected]>

gabritto and others added 3 commits August 11, 2025 13:16

basic jsdoc completions

6f4c926

jsdoc completion details

d0fe474

Merge branch 'main' into gabritto/jsdoccompletion

1c139db

jakebailey reviewed Aug 11, 2025

View reviewed changes

internal/ls/completions.go Show resolved Hide resolved

gabritto and others added 5 commits August 12, 2025 18:48

add test for parameter jsdoc case and fix bugs

5681cd2

update failing

586cc88

Merge branch 'main' into gabritto/jsdoccompletion

7195326

uncomment test

735e964

remove comment

dbb019c

gabritto marked this pull request as ready for review August 13, 2025 02:17

Copilot AI review requested due to automatic review settings August 13, 2025 02:17

Copilot AI reviewed Aug 13, 2025

View reviewed changes

fix bad spelling

9f591cd

gabritto requested review from jakebailey and sandersn August 13, 2025 02:18

jakebailey reviewed Aug 13, 2025

View reviewed changes

internal/format/util.go Show resolved Hide resolved

jakebailey reviewed Aug 13, 2025

View reviewed changes

internal/ls/completions.go Outdated Show resolved Hide resolved

jakebailey reviewed Aug 13, 2025

View reviewed changes

internal/ls/completions.go Show resolved Hide resolved

access string position directly

d9fdf3b

jakebailey reviewed Aug 13, 2025

View reviewed changes

internal/checker/printer.go Show resolved Hide resolved

update test with case that has param initializer

29a2e97

jakebailey approved these changes Aug 13, 2025

View reviewed changes

sandersn approved these changes Aug 13, 2025

View reviewed changes

gabritto and others added 4 commits August 13, 2025 16:15

Apply suggestions from code review

f2d246d

Co-authored-by: Nathan Shively-Sanders <[email protected]>

Object -> object

ec2c9bb

rename and move bad jsdoc function

cb9e480

update test

ec55a18

jakebailey approved these changes Aug 14, 2025

View reviewed changes

gabritto enabled auto-merge August 14, 2025 16:44

gabritto added this pull request to the merge queue Aug 14, 2025

Merged via the queue into main with commit 73637ac Aug 14, 2025
22 checks passed

gabritto deleted the gabritto/jsdoccompletion branch August 14, 2025 17:02

andrewbranch pushed a commit to andrewbranch/typescript-go that referenced this pull request Aug 18, 2025

JSDoc completions (microsoft#1561)

fc34cd0

Co-authored-by: Nathan Shively-Sanders <[email protected]>

JSDoc completions #1561

JSDoc completions #1561

Uh oh!

Conversation

gabritto commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

jakebailey commented Aug 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabritto Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gabritto commented Aug 11, 2025 •

edited

Loading

gabritto Aug 13, 2025 •

edited

Loading